Skip to content

Add some comments to zend_gc.c #19412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tstarling
Copy link
Contributor

It took me a while to understand this code, so it makes sense to write some notes and share them.

It took me a while to understand this code, so it makes sense to write
some notes and share them.
Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Zend/zend_gc.c Outdated
Comment on lines 1261 to 1262
/* For all roots marked purple, traverse the graph, marking referred objects grey.
* See MarkRoots() in Bacon & Rajan. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, for each traversed node, this decrements the refcount of child nodes. The purpose of marking grey is to remember which nodes where already traversed during this step.

On a higher level this simulates the effect (on refcounts) of deleting roots. Nodes that end up with a refcount > 0, and nodes reachable from them, are proven to not be part of a garbage cycle, and to be live. gc_scan_roots() will then restore the refcount of live nodes and mark other ones as garbage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I clarified this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/* For all roots marked purple, traverse the graph, decrementing the reference
 * count of visited nodes. Mark visited nodes grey so that their reference
 * counts will only be decremented once. See MarkRoots() in Bacon & Rajan. */

This is slightly incorrect as it can be understood that we are decrementing the refcount of non-grey nodes themselves. This would also imply that we decrement the refcount at most once per node. In reality we decrement the refcount of their child nodes ("internal reference counts" in the paper).

I suggest:

For all roots marked purple, traverse the graph, decrementing the reference
count of their child nodes. Mark visited nodes grey so that they are not
visited again. See MarkRoots() in Bacon & Rajan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants